Skip to content

Add E2E tests for keyword routing (Issue #667) #684

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Nov 18, 2025
Merged

Add E2E tests for keyword routing (Issue #667) #684

merged 1 commit into from
Nov 18, 2025

Conversation

@szedan-rh
Copy link
Contributor

@szedan-rh szedan-rh commented Nov 17, 2025

Summary

This PR implements comprehensive end-to-end tests for keyword routing functionality, addressing Issue #667.

Test Coverage

35 Tests Covering:

  • OR operator (6 tests): match when any keyword is present
  • AND operator (5 tests): match when all keywords must be present
  • NOR operator (2 tests): match when no keywords are present
  • Case sensitivity (3 tests): case-sensitive and case-insensitive matching
  • Word boundaries (3 tests): prevent partial word matches
  • Regex special characters (3 tests): handle dots, asterisks literally
  • Edge cases (8 tests): empty text, Unicode, emoji, newlines
  • Multiple rule matching (2 tests): priority order validation
  • Confidence scores (1 test): keyword matches return 1.0
  • JSON test data (1 test): load and validate test cases
  • Error handling (2 tests): invalid operators, empty keywords

Test Results

Ran 35 of 35 Specs in 0.012 seconds
✅ 35 Passed | ❌ 0 Failed | ⏭️ 0 Pending | ⏩ 0 Skipped
PASS

Performance:

  • Total: 35 tests
  • Average test time: < 0.3ms per test
  • Race detection: PASS (no race conditions)

Files Added

  • e2e-tests/testcases/suite_test.go: Ginkgo test suite entry point
  • e2e-tests/testcases/keyword_routing_test.go: 35 comprehensive tests
  • e2e-tests/testcases/helpers.go: Test helper functions
  • e2e-tests/testcases/testdata/keyword_routing_cases.json: 26 JSON test cases
  • e2e-tests/testcases/go.mod & go.sum: Module configuration
  • .github/workflows/unit-test-e2e-testcases.yml: CI/CD workflow

Technical Highlights

NOR Operator Isolation

Created dedicated classifier for NOR tests to prevent false matches in tests expecting empty results.

Test Framework

  • Uses Ginkgo v2 + Gomega for BDD-style testing
  • Table-driven tests for similar test cases
  • JSON test data for easy maintenance
  • Proper module setup with replace directives

CI Integration

GitHub Actions workflow includes:

  • Keyword routing tests
  • Race condition detection
  • Code linting
  • Coverage reporting to Codecov

Checklist

  • All tests pass locally
  • No race conditions detected
  • Code passes linting (go fmt applied)
  • Test data in JSON format
  • Helper functions documented
  • CI workflow configured

Related Issues

Fixes #667

Signed-off-by: szedan

@netlify
Copy link

netlify bot commented Nov 17, 2025 •
edited
Loading

✅ Deploy Preview for vllm-semantic-router ready!

Name Link
🔨 Latest commit 7672d1f
🔍 Latest deploy log https://app.netlify.com/projects/vllm-semantic-router/deploys/691caf7423cecb0008e16d29
😎 Deploy Preview https://deploy-preview-684--vllm-semantic-router.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@szedan-rh szedan-rh marked this pull request as draft November 17, 2025 15:31
@codecov-commenter
Copy link

codecov-commenter commented Nov 17, 2025

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@szedan-rh szedan-rh force-pushed the test_keyword_routing branch from b0dfb6a to 7d27e11 Compare November 17, 2025 17:15
@szedan-rh szedan-rh marked this pull request as ready for review November 18, 2025 12:42
@szedan-rh
Copy link
Contributor Author

szedan-rh commented Nov 18, 2025 •
edited
Loading

Hi @Xunzhuo ,
This is the first PR for test keyword routing, could you please review?
Another PRs for the next tests will be raised separately

@rootfs
Copy link
Collaborator

rootfs commented Nov 18, 2025

@srini-abhiram PTAL, thanks

@github-actions
Copy link

github-actions bot commented Nov 18, 2025 •
edited
Loading

👥 vLLM Semantic Team Notification

The following members have been identified for the changed files in this PR and have been automatically assigned:

📁 Root Directory

Owners: @rootfs, @Xunzhuo
Files changed:

  • .github/workflows/unit-test-e2e-testcases.yml

📁 e2e-tests

Owners: @yossiovadia
Files changed:

  • e2e-tests/testcases/go.mod
  • e2e-tests/testcases/go.sum
  • e2e-tests/testcases/helpers.go
  • e2e-tests/testcases/keyword_routing_test.go
  • e2e-tests/testcases/suite_test.go
  • e2e-tests/testcases/testdata/keyword_routing_cases.json
vLLM

🎉 Thanks for your contributions!

This comment was automatically generated based on the OWNER files in the repository.

@szedan-rh szedan-rh force-pushed the test_keyword_routing branch from e1a1311 to 6a09f09 Compare November 18, 2025 17:27
@szedan-rh
Add E2E tests for keyword routing
7672d1f 
Implement comprehensive end-to-end tests for keyword routing functionality
to address Issue vllm-project#667.

Test Coverage:
- OR operator: any keyword matches
- AND operator: all keywords must match
- NOR operator: no keywords match
- Case-sensitive vs case-insensitive matching
- Regex pattern matching and special character handling
- Word boundary detection
- Edge cases: empty text, Unicode, emoji, punctuation
- Multiple rule matching and priority
- Confidence score validation
- Error handling

Implementation Details:
- 35 comprehensive tests using Ginkgo v2 and Gomega
- JSON test data files for maintainability (26 test cases)
- Helper functions for test setup
- Isolated NOR operator tests to prevent false matches
- CI/CD integration with GitHub Actions workflow
- Coverage measurement: 87.1% of keyword_classifier.go (exceeds 80% threshold)
- Race condition detection
- golangci-lint integration

Technical Fixes:
- Removed unused helper functions to pass linter
- Disabled CUDA features for CI environment (--no-default-features)
- Fixed coverage reporting to measure keyword_classifier.go specifically
- Removed -run filter that doesn't work with Ginkgo tests

Files Added:
- .github/workflows/unit-test-e2e-testcases.yml
- e2e-tests/testcases/suite_test.go
- e2e-tests/testcases/keyword_routing_test.go
- e2e-tests/testcases/helpers.go
- e2e-tests/testcases/testdata/keyword_routing_cases.json
- e2e-tests/testcases/go.mod
- e2e-tests/testcases/go.sum

Signed-off-by: Senan Zedan <[email protected]>
@szedan-rh szedan-rh force-pushed the test_keyword_routing branch from 6a09f09 to 7672d1f Compare November 18, 2025 17:40
@szedan-rh
Copy link
Contributor Author

szedan-rh commented Nov 18, 2025

After sync the branch - fix back all the failing unit tests and squash all commits to one commit for easy code review.

@rootfs rootfs merged commit 1c8c932 into vllm-project:main Nov 18, 2025
24 checks passed
@Xunzhuo
Copy link
Member

Xunzhuo commented Nov 19, 2025 •
edited
Loading

@szedan-rh i think the testcases is located incorrectly, plz move the cases into e2e/testcases, this will trigger the e2e tests in CI, plz read other testcases to understand how it works

@szedan-rh
Copy link
Contributor Author

szedan-rh commented Nov 19, 2025

You Right, I put them under e2e-tests.
will change that

@srini-abhiram
Copy link
Contributor

srini-abhiram commented Nov 19, 2025 •
edited
Loading

There are some un unused types in helpers.go such as EmbeddingTestCase, HybridTestCase.
Also, the matched_keywords field in keyword_routing_cases.json is not asserted in the test cases, currently only the matched category is asserted. This might reproduce the incorrect matching bug, for example: star in stardom.

@szedan-rh
Copy link
Contributor Author

szedan-rh commented Nov 19, 2025

@srini-abhiram - it's there because i will raise soon pr's for those test cases, wanted to do that one by one. seems forogt to remove them before raising this PR.
will handle that also.

@szedan-rh
Copy link
Contributor Author

szedan-rh commented Nov 19, 2025

@Xunzhuo / @srini-abhiram - Could you please take another look?
I revert the changes in the helpers.go and moved that testcases to the correct path
#694
(please note that PRs that we raising me and @yehudit1987 ) is failing on lint issue related to .md files
Don't have reason why it start to fail now but should be handled in different PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rootfs rootfs Awaiting requested review from rootfs rootfs is a code owner

@Xunzhuo Xunzhuo Awaiting requested review from Xunzhuo Xunzhuo is a code owner

Assignees

@yossiovadia yossiovadia

@rootfs rootfs

@Xunzhuo Xunzhuo

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[E2E Testing] Add comprehensive test coverage for routing strategies and filters

6 participants

@szedan-rh @codecov-commenter @rootfs @Xunzhuo @srini-abhiram @yossiovadia